fix: return 409 from set-workspace when multiple WSGI workers are in use#150
Conversation
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 22 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughThe workspace override endpoint now detects multi-worker deployments and returns HTTP 409 with a stable error code instead of applying a process-local override. The configuration UI handles this response, tests cover detection and endpoint behavior, and deployment documentation specifies startup configuration. ChangesMulti-worker workspace safeguards
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Browser
participant set_workspace
participant WorkerDetection
Browser->>set_workspace: POST /api/set-workspace
set_workspace->>WorkerDetection: Check worker indicators
WorkerDetection-->>set_workspace: Multi-worker status
set_workspace-->>Browser: HTTP 409 error or HTTP 200 success
Browser-->>Browser: Show error or continue redirect
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
@CodeRabbit, now can you start a revview about this pr? still limited to review? |
|
✅ Action performedReview finished.
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
bradjin8
left a comment
bradjin8
left a comment
There was a problem hiding this comment.
Nice to have: handle 409 in config page
config.html calls /api/set-workspace without checking the response status. On 409, the UI may still show success. Consider surfacing body.error / body.code when res.status === 409.
Config now checks the set-workspace response before we treat it as saved. On 409 we show the API error text and skip localStorage and the redirect. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@templates/config.html`:
- Around line 85-90: Update the auto-detect save flow after
postSetWorkspace(detected) so failed responses render saved.message through the
existing `#status-msg` error handling used by validateAndSave() before returning,
while preserving the current redirect behavior for successful saves.
In `@utils/workspace_path.py`:
- Around line 59-62: Update the worker-detection loop in the relevant
workspace-path helper so values greater than one return true, while
single-worker values continue checking later signals such as GUNICORN_WORKERS
and GUNICORN_CMD_ARGS. Add a regression test covering WEB_CONCURRENCY=1 together
with a later multi-worker signal and assert multi-worker mode is detected.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 97390ddc-a670-40ed-bcd7-20b08a46b97c
📒 Files selected for processing (5)
DEPLOYMENT.mdapi/config_api.pytemplates/config.htmltests/test_set_workspace_multiworker.pyutils/workspace_path.py
6f30b91 to
e4ca468
Compare
3 participants
Closes #144
With gunicorn running more than one worker, POST set-workspace only changed the in-memory override on the worker that handled the request.
The response still looked like success, so other workers kept the old path and operators had no signal that the change did not stick fleet-wide.
This branch blocks that case.
After path validation, the handler checks whether the deployment looks multi-worker using environment signals such as WEB_CONCURRENCY, GUNICORN_WORKERS, worker flags in GUNICORN_CMD_ARGS, or an explicit CURSOR_BROWSER_MULTI_WORKER override.
When that check is true, the API returns HTTP 409 with error code set_workspace_multi_worker_unsupported and tells the operator to set WORKSPACE_PATH or pass base-dir at startup instead of using the Configuration page. Single-process and single-worker behavior is unchanged when those signals are absent.
DEPLOYMENT.md now describes the 409 response and the env vars involved. tests/test_set_workspace_multiworker.py covers the 409 path, the success path when detection is off, and the env parsing helpers.
Sanity check by running that test module and the existing set-workspace API tests in test_workspace_path_validation.py.
Summary by CodeRabbit
Bug Fixes
Documentation
POST /api/set-workspace.Tests
/api/set-workspace409 vs 200 outcomes.